Unverified Commit 89888a47 authored by Alexander Maryanovsky's avatar Alexander Maryanovsky Committed by GitHub

Fix race condition reading layer properties in a background thread (#1181)

parent 901d632a
......@@ -645,13 +645,22 @@ actual open class SkiaLayer internal constructor(
@Suppress("LeakingThis")
private val fpsCounter = defaultFPSCounter(this)
internal inline fun inDrawScope(body: () -> Unit) {
private fun createDrawScope() = LayerDrawScope(
pixelGeometry = pixelGeometry,
layerWidth = width,
layerHeight = height,
scale = contentScale
)
internal inline fun inDrawScope(body: LayerDrawScope.() -> Unit) {
check(isEventDispatchThread()) { "Method should be called from AWT event dispatch thread" }
check(!isDisposed) { "SkiaLayer is disposed" }
try {
fpsCounter?.tick()
body()
} catch (e: CancellationException) {
with(createDrawScope()) {
body()
}
} catch (_: CancellationException) {
// ignore
} catch (e: RenderException) {
if (!isDisposed) {
......
......@@ -2,6 +2,7 @@ package org.jetbrains.skiko.context
import org.jetbrains.skia.*
import org.jetbrains.skiko.AngleApi
import org.jetbrains.skiko.LayerDrawScope
import org.jetbrains.skiko.RenderException
import org.jetbrains.skiko.SkiaLayer
import org.jetbrains.skiko.redrawer.AngleRedrawer
......@@ -23,12 +24,11 @@ internal class AngleContextHandler(layer: SkiaLayer) : ContextBasedContextHandle
return false
}
override fun initCanvas() {
override fun LayerDrawScope.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()
......@@ -41,7 +41,7 @@ internal class AngleContextHandler(layer: SkiaLayer) : ContextBasedContextHandle
SurfaceOrigin.BOTTOM_LEFT,
SurfaceColorFormat.RGBA_8888,
ColorSpace.sRGB,
SurfaceProps(pixelGeometry = layer.pixelGeometry)
SurfaceProps(pixelGeometry = pixelGeometry)
) ?: throw RenderException("Cannot create surface")
}
......
......@@ -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 {
......
......@@ -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
}
}
\ No newline at end of file
......@@ -4,7 +4,7 @@ import org.jetbrains.skia.DirectContext
import org.jetbrains.skia.Surface
import org.jetbrains.skia.SurfaceProps
import org.jetbrains.skia.impl.getPtr
import org.jetbrains.skiko.Logger
import org.jetbrains.skiko.LayerDrawScope
import org.jetbrains.skiko.SkiaLayer
import org.jetbrains.skiko.redrawer.Direct3DRedrawer
import java.lang.ref.Reference
......@@ -30,15 +30,14 @@ internal class Direct3DContextHandler(layer: SkiaLayer) : ContextBasedContextHan
return false
}
override fun initCanvas() {
override fun LayerDrawScope.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()
......@@ -46,7 +45,7 @@ internal class Direct3DContextHandler(layer: SkiaLayer) : ContextBasedContextHan
val justInitialized = directXRedrawer.changeSize(width, height)
try {
val surfaceProps = SurfaceProps(pixelGeometry = layer.pixelGeometry)
val surfaceProps = SurfaceProps(pixelGeometry = pixelGeometry)
for (bufferIndex in 0 until bufferCount) {
surfaces[bufferIndex] = directXRedrawer.makeSurface(
context = getPtr(context),
......@@ -68,7 +67,7 @@ internal class Direct3DContextHandler(layer: SkiaLayer) : ContextBasedContextHan
canvas = surface!!.canvas
}
override fun flush() {
override fun flush(scope: LayerDrawScope) {
val context = context ?: return
val surface = surface ?: return
try {
......
package org.jetbrains.skiko.context
import org.jetbrains.skia.impl.getPtr
import org.jetbrains.skiko.LayerDrawScope
import org.jetbrains.skiko.SkiaLayer
import org.jetbrains.skiko.redrawer.AbstractDirectSoftwareRedrawer
import java.lang.ref.Reference
......@@ -20,10 +21,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 LayerDrawScope.initCanvas() {
val w = scaledLayerWidth
val h = scaledLayerHeight
if (isSizeChanged(w, h) || surface == null) {
disposeCanvas()
if (w > 0 && h > 0) {
......@@ -37,7 +37,7 @@ internal class DirectSoftwareContextHandler(layer: SkiaLayer) : ContextFreeConte
}
}
override fun flush() {
override fun flush(scope: LayerDrawScope) {
val surface = surface
if (surface != null) {
try {
......
package org.jetbrains.skiko.context
import org.jetbrains.skia.*
import org.jetbrains.skiko.LayerDrawScope
import org.jetbrains.skiko.Logger
import org.jetbrains.skiko.MetalAdapter
import org.jetbrains.skiko.RenderException
......@@ -20,12 +21,11 @@ internal class MetalContextHandler(
private val device: MetalDevice,
private val adapter: MetalAdapter
) : ContextBasedContextHandler(layer, "Metal") {
override fun initCanvas() {
override fun LayerDrawScope.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)
......@@ -36,7 +36,7 @@ internal class MetalContextHandler(
SurfaceOrigin.TOP_LEFT,
SurfaceColorFormat.BGRA_8888,
ColorSpace.sRGB,
SurfaceProps(pixelGeometry = layer.pixelGeometry)
SurfaceProps(pixelGeometry = pixelGeometry)
) ?: throw RenderException("Cannot create surface")
canvas = surface!!.canvas
......@@ -47,8 +47,8 @@ internal class MetalContextHandler(
}
}
override fun flush() {
super.flush()
override fun flush(scope: LayerDrawScope) {
super.flush(scope)
surface?.flushAndSubmit()
finishFrame()
Logger.debug { "MetalContextHandler finished drawing frame" }
......
......@@ -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 LayerDrawScope.initCanvas() {
val w = scaledLayerWidth
val h = scaledLayerHeight
if (isSizeChanged(w, h) || surface == null) {
disposeCanvas()
......@@ -41,7 +40,7 @@ internal class OpenGLContextHandler(layer: SkiaLayer) : ContextBasedContextHandl
SurfaceOrigin.BOTTOM_LEFT,
SurfaceColorFormat.RGBA_8888,
ColorSpace.sRGB,
SurfaceProps(pixelGeometry = layer.pixelGeometry)
SurfaceProps(pixelGeometry = pixelGeometry)
) ?: throw RenderException("Cannot create surface")
}
......
package org.jetbrains.skiko.context
import org.jetbrains.skia.*
import org.jetbrains.skiko.LayerDrawScope
import org.jetbrains.skiko.OS
import org.jetbrains.skiko.SkiaLayer
import org.jetbrains.skiko.hostOs
......@@ -22,25 +23,22 @@ internal class SoftwareContextHandler(layer: SkiaLayer) : ContextFreeContextHand
var imageData: ByteArray? = null
var raster: WritableRaster? = null
override fun initCanvas() {
override fun LayerDrawScope.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)
}
canvas = Canvas(storage, SurfaceProps(pixelGeometry = layer.pixelGeometry))
canvas = Canvas(storage, SurfaceProps(pixelGeometry = pixelGeometry))
}
override fun flush() {
val scale = layer.contentScale
val w = (layer.width * scale).toInt().coerceAtLeast(0)
val h = (layer.height * scale).toInt().coerceAtLeast(0)
override fun flush(scope: LayerDrawScope) {
val w = scope.scaledLayerWidth
val h = scope.scaledLayerHeight
val bytes = storage.readPixels(storage.imageInfo, (w * 4), 0, 0)
if (bytes != null) {
......
......@@ -54,7 +54,7 @@ internal abstract class AWTRedrawer(
layer.update(nanoTime)
}
protected inline fun inDrawScope(body: () -> Unit) {
protected inline fun inDrawScope(body: LayerDrawScope.() -> Unit) {
requireNotNull(deviceAnalytics) { "deviceAnalytics is not null. Call onDeviceChosen after choosing the drawing device" }
if (!isDisposed) {
val isFirstFrame = !isFirstFrameRendered
......
......@@ -33,7 +33,7 @@ internal abstract class AbstractDirectSoftwareRedrawer(
frameDispatcher.scheduleFrame()
}
protected open fun draw() = inDrawScope(contextHandler::draw)
protected open fun draw() = inDrawScope { contextHandler.draw() }
override fun renderImmediately() {
update()
......
......@@ -89,7 +89,9 @@ internal class AngleRedrawer(
return
}
makeCurrent(device)
contextHandler.draw()
layer.inDrawScope {
contextHandler.draw()
}
swapBuffers(device, withVsync)
}
......
......@@ -86,7 +86,7 @@ internal class Direct3DRedrawer(
}
}
private fun drawAndSwap(withVsync: Boolean) = synchronized(drawLock) {
private fun LayerDrawScope.drawAndSwap(withVsync: Boolean) = synchronized(drawLock) {
if (isDisposed) {
return
}
......
......@@ -97,7 +97,7 @@ internal class LinuxOpenGLRedrawer(
}
private fun draw() {
inDrawScope(contextHandler::draw)
inDrawScope { contextHandler.draw() }
}
companion object {
......
......@@ -144,7 +144,7 @@ internal class MetalRedrawer(
windowOcclusionStateChannel.trySend(isOccluded)
}
private fun performDraw() = synchronized(drawLock) {
private fun LayerDrawScope.performDraw() = synchronized(drawLock) {
if (!isDisposed) {
autoreleasepool {
contextHandler.draw()
......
......@@ -27,7 +27,7 @@ internal class SoftwareRedrawer(
if (layer.isShowing) {
update()
inDrawScope(contextHandler::draw)
inDrawScope { contextHandler.draw() }
}
}
......
......@@ -78,7 +78,7 @@ internal class WindowsOpenGLRedrawer(
}
private fun draw() {
inDrawScope(contextHandler::draw)
inDrawScope { contextHandler.draw() }
}
private fun makeCurrent() = makeCurrent(device, context)
......
......@@ -578,9 +578,9 @@ class SkiaLayerTest {
object : BaseTestRedrawer(layer) {
private val contextHandler = object : JvmContextHandler(layer) {
override fun initContext() = false
override fun initCanvas() = Unit
override fun LayerDrawScope.initCanvas() = Unit
}
override fun renderImmediately() = layer.inDrawScope(contextHandler::draw)
override fun renderImmediately() = layer.inDrawScope { contextHandler.draw() }
}
}
}
......
......@@ -3,6 +3,7 @@ package org.jetbrains.skiko
import org.jetbrains.skia.Canvas
import org.jetbrains.skia.Picture
import org.jetbrains.skia.PixelGeometry
import org.jetbrains.skiko.context.ContextHandler
/**
* Generic layer for Skiko rendering.
......@@ -70,6 +71,35 @@ expect open class SkiaLayer {
internal fun draw(canvas: Canvas)
}
internal class PictureHolder(val instance: Picture, val width: Int, val height: Int)
internal class LayerDrawScope(
val pixelGeometry: PixelGeometry,
val scaledLayerWidth: Int,
val scaledLayerHeight: Int,
) {
constructor(
pixelGeometry: PixelGeometry,
layerWidth: Int,
layerHeight: Int,
scale: Float
): this(
pixelGeometry = pixelGeometry,
scaledLayerWidth = (layerWidth * scale).toInt().coerceAtLeast(0),
scaledLayerHeight = (layerHeight * scale).toInt().coerceAtLeast(0)
)
constructor(
pixelGeometry: PixelGeometry,
layerWidth: Double,
layerHeight: Double,
scale: Float
): this(
pixelGeometry = pixelGeometry,
scaledLayerWidth = (layerWidth * scale).toInt().coerceAtLeast(0),
scaledLayerHeight = (layerHeight * scale).toInt().coerceAtLeast(0)
)
internal fun ContextHandler.draw() {
this@LayerDrawScope.draw()
}
}
\ No newline at end of file
......@@ -13,9 +13,9 @@ internal abstract class ContextHandler(
protected var canvas: Canvas? = null
protected abstract fun initContext(): Boolean
protected abstract fun initCanvas()
protected abstract fun LayerDrawScope.initCanvas()
protected open fun flush() {
protected open fun flush(scope: LayerDrawScope) {
context?.flush()
}
......@@ -35,7 +35,7 @@ internal abstract class ContextHandler(
}
// throws RenderException if initialization of graphic context was not successful
fun draw() {
fun LayerDrawScope.draw() {
if (!initContext()) {
throw RenderException("Cannot init graphic context")
}
......@@ -44,6 +44,7 @@ internal abstract class ContextHandler(
clear(Color.TRANSPARENT)
drawContent()
}
flush()
flush(this)
}
}
\ No newline at end of file
}
......@@ -168,4 +168,17 @@ actual open class SkiaLayer {
actual val pixelGeometry: PixelGeometry
get() = PixelGeometry.UNKNOWN
private fun createDrawScope() = LayerDrawScope(
pixelGeometry = pixelGeometry,
layerWidth = nsView.frame.useContents { size.width },
layerHeight = nsView.frame.useContents { size.height },
scale = contentScale
)
internal fun inDrawScope(block: LayerDrawScope.() -> Unit) {
with(createDrawScope()) {
block()
}
}
}
package org.jetbrains.skiko.context
import kotlinx.cinterop.useContents
import org.jetbrains.skia.*
import org.jetbrains.skiko.LayerDrawScope
import org.jetbrains.skiko.RenderException
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,11 @@ internal class MacOsMetalContextHandler(layer: SkiaLayer) : ContextHandler(layer
return true
}
override fun initCanvas() {
override fun LayerDrawScope.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)
......@@ -52,9 +51,9 @@ internal class MacOsMetalContextHandler(layer: SkiaLayer) : ContextHandler(layer
}
}
override fun flush() {
override fun flush(scope: LayerDrawScope) {
// TODO: maybe make flush async as in JVM version.
super.flush()
super.flush(scope)
surface?.flushAndSubmit()
metalRedrawer.finishFrame()
}
......
......@@ -3,6 +3,7 @@ package org.jetbrains.skiko.context
import kotlinx.cinterop.*
import org.jetbrains.skia.*
import org.jetbrains.skiko.GraphicsApi
import org.jetbrains.skiko.LayerDrawScope
import org.jetbrains.skiko.RenderException
import org.jetbrains.skiko.SkiaLayer
import platform.OpenGL.GL_DRAW_FRAMEBUFFER_BINDING
......@@ -10,7 +11,8 @@ import platform.OpenGL.glGetIntegerv
import platform.OpenGLCommon.GLenum
/**
* OpenGL context handler for MacOs (native).
* OpenGL context handler for macOS (native).
*
* Not used anymore, unless corresponding [GraphicsApi] is hardcoded in [SkiaLayer].
* See [MacOsMetalContextHandler] instead.
*/
......@@ -20,7 +22,7 @@ internal class MacOSOpenGLContextHandler(layer: SkiaLayer) : ContextHandler(laye
if (context == null) {
context = DirectContext.makeGL()
}
} catch (e: Exception) {
} catch (_: Exception) {
println("Failed to create Skia OpenGL context!")
return false
}
......@@ -29,11 +31,11 @@ internal class MacOSOpenGLContextHandler(layer: SkiaLayer) : ContextHandler(laye
@ExperimentalUnsignedTypes
private fun openglGetIntegerv(pname: GLenum): UInt {
var result: UInt = 0U
var result = 0U
memScoped {
val data = alloc<IntVar>()
glGetIntegerv(pname, data.ptr);
result = data.value.toUInt();
glGetIntegerv(pname, data.ptr)
result = data.value.toUInt()
}
return result
}
......@@ -49,10 +51,9 @@ internal class MacOSOpenGLContextHandler(layer: SkiaLayer) : ContextHandler(laye
return false
}
override fun initCanvas() {
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)
override fun LayerDrawScope.initCanvas() {
val w = scaledLayerWidth
val h = scaledLayerHeight
if (isSizeChanged(w, h)) {
val fbId = openglGetIntegerv(GL_DRAW_FRAMEBUFFER_BINDING.toUInt())
renderTarget = BackendRenderTarget.makeGL(
......
......@@ -157,7 +157,9 @@ internal class MacOsMetalRedrawer(
update()
}
if (!isDisposed) { // Redrawer may be disposed in user code, during `update`
contextHandler.draw()
skiaLayer.inDrawScope {
contextHandler.draw()
}
}
}
}
......@@ -166,7 +168,9 @@ internal class MacOsMetalRedrawer(
autoreleasepool {
if (!isDisposed) {
update()
contextHandler.draw()
skiaLayer.inDrawScope {
contextHandler.draw()
}
}
}
......@@ -233,6 +237,8 @@ internal class MetalLayer : CAMetalLayer {
override fun drawInContext(ctx: CGContextRef?) {
skiaLayer.update(currentNanoTime())
contextHandler.draw()
skiaLayer.inDrawScope {
contextHandler.draw()
}
}
}
......@@ -133,7 +133,9 @@ internal class MacosGLLayer : CAOpenGLLayer {
CGLSetCurrentContext(ctx)
try {
skiaLayer.update(currentNanoTime())
contextHandler.draw()
skiaLayer.inDrawScope {
contextHandler.draw()
}
} catch (e: Throwable) {
e.printStackTrace()
throw e
......
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