Fix for losing presented drawables on JVM Mac OS (#708)

Implement machinery for blocking draw dispatch before next vsync using synchronous wait on semaphore in native code.
Implement mechanism to prevent Metal command buffers overcommitment.

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

@@ -73,8 +73,6 @@ internal class MetalRedrawer(
 
     override val renderInfo: String get() = contextHandler.rendererInfo()
 
-    private val windowHandle = layer.windowHandle
-
     private val frameDispatcher = FrameDispatcher(MainUIDispatcher) {
         if (layer.isShowing) {
             update(System.nanoTime())
@@ -86,6 +84,10 @@ internal class MetalRedrawer(
         onContextInit()
     }
 
+    fun drawSync() {
+        frameDispatcher.scheduleFrame()
+    }
+
     override fun dispose() = synchronized(drawLock) {
         frameDispatcher.cancel()
         contextHandler.dispose()
@@ -133,10 +135,13 @@ internal class MetalRedrawer(
         // When window is not visible - it doesn't make sense to redraw fast to avoid battery drain.
         // In theory, we could be more precise, and just suspend rendering in
         // `NSWindowDidChangeOcclusionStateNotification`, but current approach seems to work as well in practise.
-        if (isOccluded(windowHandle))
+        if (isOccluded())
             delay(300)
     }
 
+    /**
+     * Avoid calling this method on main thread, it will sleep before next vsync happens if invoked multiple times.
+     */
     private fun performDraw() = synchronized(drawLock) {
         if (!isDisposed) {
             val handle = startRendering()
@@ -166,6 +171,8 @@ internal class MetalRedrawer(
         setLayerVisible(device.ptr, isVisible)
     }
 
+    private fun isOccluded() = isOccluded(device.ptr)
+
     private external fun chooseAdapter(adapterPriority: Int): Long
     private external fun createMetalDevice(window:Long, transparency: Boolean, adapter: Long, platformInfo: Long): Long
     private external fun disposeDevice(device: Long)
@@ -173,7 +180,7 @@ internal class MetalRedrawer(
     private external fun setLayerVisible(device: Long, isVisible: Boolean)
     private external fun setContentScale(device: Long, contentScale: Float)
     private external fun setVSyncEnabled(device: Long, enabled: Boolean)
-    private external fun isOccluded(window: Long): Boolean
+    private external fun isOccluded(device: Long): Boolean
     private external fun getAdapterName(adapter: Long): String
     private external fun getAdapterMemorySize(adapter: Long): Long
     private external fun startRendering(): Long

skiko/src/awtMain/objectiveC/macos/AWTMetalLayer.h

+#ifdef SK_METAL
+
+#ifndef SK_AWT_METAL_LAYER_H
+#define SK_AWT_METAL_LAYER_H
+
+#import <QuartzCore/CAMetalLayer.h>
+#import <jni.h>
+
+@interface AWTMetalLayer : CAMetalLayer
+
+@property jobject javaRef;
+
+@end
+
+#endif // SK_AWT_METAL_LAYER_H
+
+#endif // SK_METAL
\ No newline at end of file

skiko/src/awtMain/objectiveC/macos/AWTMetalLayer.mm

+#import "AWTMetalLayer.h"
+
+@implementation AWTMetalLayer
+
+- (id)init {
+    self = [super init];
+
+    if (self) {
+        [self removeAllAnimations];
+        [self setAutoresizingMask: (kCALayerWidthSizable|kCALayerHeightSizable)];
+        [self setNeedsDisplayOnBoundsChange: YES];
+    }
+
+    return self;
+}
+
+@end

skiko/src/awtMain/objectiveC/macos/MetalContextHandler.mm

@@ -3,8 +3,6 @@
 #import <jawt.h>
 #import <jawt_md.h>
 
-#import <QuartzCore/CAMetalLayer.h>
-#import <Metal/Metal.h>
 #import <GrDirectContext.h>
 #import <mtl/GrMtlBackendContext.h>
 #import <mtl/GrMtlTypes.h>
@@ -13,6 +11,7 @@
 
 extern "C"
 {
+
 JNIEXPORT jlong JNICALL Java_org_jetbrains_skiko_context_MetalContextHandler_makeMetalContext(
     JNIEnv* env, jobject contextHandler, jlong devicePtr)
 {
@@ -30,10 +29,15 @@ JNIEXPORT jlong JNICALL Java_org_jetbrains_skiko_context_MetalContextHandler_mak
 {
     @autoreleasepool {
         MetalDevice *device = (__bridge MetalDevice *) (void *) devicePtr;
+        [device recreateDisplayLinkIfNeeded];
+        [device waitUntilVsync];
+        [device waitForQueueSlot];
+
         GrBackendRenderTarget* renderTarget = NULL;
 
         id<CAMetalDrawable> currentDrawable = [device.layer nextDrawable];
         if (!currentDrawable) {
+            [device freeQueueSlot];
             return NULL;
         }
         device.drawableHandle = currentDrawable;
@@ -56,6 +60,9 @@ JNIEXPORT void JNICALL Java_org_jetbrains_skiko_context_MetalContextHandler_fini
             id<MTLCommandBuffer> commandBuffer = [device.queue commandBuffer];
             commandBuffer.label = @"Present";
             [commandBuffer presentDrawable:currentDrawable];
+            [commandBuffer addCompletedHandler:^(id<MTLCommandBuffer> buffer) {
+                [device freeQueueSlot];
+            }];
             [commandBuffer commit];
             device.drawableHandle = nil;
         }

skiko/src/awtMain/objectiveC/macos/MetalDevice.h

 #ifdef SK_METAL
 
-@interface AWTMetalLayer : CAMetalLayer
+#ifndef SK_METAL_DEVICE_H
+#define SK_METAL_DEVICE_H
 
-@property jobject javaRef;
+#import <QuartzCore/QuartzCore.h>
+#import <AppKit/AppKit.h>
+#import <Metal/Metal.h>
 
-@end
+#import "AWTMetalLayer.h"
 
 @interface MetalDevice : NSObject
 
 @property (weak) CALayer *container;
-@property (retain, strong) AWTMetalLayer *layer;
-@property (retain, strong) id<MTLDevice> adapter;
-@property (retain, strong) id<MTLCommandQueue> queue;
-@property (retain, strong) id<CAMetalDrawable> drawableHandle;
+@property (strong) NSWindow *window;
+@property (strong) AWTMetalLayer *layer;
+@property (strong) id<MTLDevice> adapter;
+@property (strong) id<MTLCommandQueue> queue;
+@property (strong) id<CAMetalDrawable> drawableHandle;
+
+- (instancetype)initWithContainer:(CALayer *)container adapter:(id<MTLDevice>)adapter window:(NSWindow *)window;
+- (void)recreateDisplayLinkIfNeeded;
+- (void)handleDisplayLinkFired;
+- (void)waitUntilVsync;
+- (void)waitForQueueSlot;
+- (void)freeQueueSlot;
 
 @end
 
-#endif
+#endif // SK_AWT_METAL_DEVICE_H
\ No newline at end of file
+
+#endif // SK_METAL
\ No newline at end of file

skiko/src/awtMain/objectiveC/macos/MetalDevice.mm

+#import "MetalDevice.h"
+#import <stdatomic.h>
+
+static CVReturn MetalDeviceDisplayLinkCallback(CVDisplayLinkRef displayLink, const CVTimeStamp *now, const CVTimeStamp *outputTime, CVOptionFlags flagsIn, CVOptionFlags *flagsOut, void *displayLinkContext) {
+    MetalDevice *device = (__bridge MetalDevice *)displayLinkContext;
+
+    [device handleDisplayLinkFired];
+
+    return kCVReturnSuccess;
+}
+
+@implementation MetalDevice {
+    NSScreen *_displayLinkScreen;
+    CVDisplayLinkRef _displayLink;
+
+    dispatch_semaphore_t _presentingBuffersExhaustionSemaphore;
+    NSConditionLock *_vsyncConditionLock;
+    volatile atomic_bool _displayLinkOk;
+}
+
+- (instancetype)initWithContainer:(CALayer *)container adapter:(id<MTLDevice>)adapter window:(NSWindow *)window {
+    self = [super init];
+
+    if (self) {
+        self.container = container;
+        self.adapter = adapter;
+        self.queue = [adapter newCommandQueue];
+        self.window = window;
+        self.layer = [AWTMetalLayer new];
+
+        _displayLinkScreen = nil;
+
+        [container removeAllAnimations];
+        [container setAutoresizingMask: (kCALayerWidthSizable|kCALayerHeightSizable)];
+        [container setNeedsDisplayOnBoundsChange: YES];
+
+        self.layer.device = adapter;
+        self.layer.pixelFormat = MTLPixelFormatBGRA8Unorm;
+        self.layer.contentsGravity = kCAGravityTopLeft;
+        CGFloat transparent[] = { 0.0f, 0.0f, 0.0f, 0.0f };
+        self.layer.backgroundColor = CGColorCreate(CGColorSpaceCreateDeviceRGB(), transparent);
+        self.layer.opaque = NO;
+
+        [container addSublayer: self.layer];
+
+        _presentingBuffersExhaustionSemaphore = dispatch_semaphore_create(3);
+
+        _vsyncConditionLock = [[NSConditionLock alloc] initWithCondition: 1];
+
+        atomic_store(&_displayLinkOk, true);
+    }
+
+    return self;
+}
+
+- (void)dealloc {
+    [self invalidateDisplayLink];
+}
+
+- (void)handleDisplayLinkSetupFailure {
+    atomic_store(&_displayLinkOk, false);
+
+    /// Next line is here to tackle edge case where displayLink reconstruction failed
+    /// but someone is already waiting at `waitUntilVsync`
+    /// It's quite improbable scenario anyway
+    [self handleDisplayLinkFired];
+}
+
+- (void)recreateDisplayLinkIfNeeded {
+    if ([self.window.screen isEqualTo: _displayLinkScreen]) {
+        return;
+    }
+
+    [self invalidateDisplayLink];
+
+    _displayLinkScreen = self.window.screen;
+
+    NSDictionary* screenDescription = [_displayLinkScreen deviceDescription];
+    NSNumber* screenID = [screenDescription objectForKey:@"NSScreenNumber"];
+
+    /// TODO: create fallback for any possible failure
+    CVReturn result;
+
+    CVDisplayLinkRef displayLink;
+    result = CVDisplayLinkCreateWithCGDisplay([screenID unsignedIntValue], &displayLink);
+
+    if (result != kCVReturnSuccess) {
+        [self handleDisplayLinkSetupFailure];
+        return;
+    }
+
+    result = CVDisplayLinkSetOutputCallback(displayLink, &MetalDeviceDisplayLinkCallback, (__bridge void *)(self));
+
+    if (result != kCVReturnSuccess) {
+        [self handleDisplayLinkSetupFailure];
+        return;
+    }
+
+    _displayLink = displayLink;
+
+    result = CVDisplayLinkStart(displayLink);
+
+    if (result != kCVReturnSuccess) {
+        [self handleDisplayLinkSetupFailure];
+        return;
+    }
+
+    atomic_store(&_displayLinkOk, true);
+
+    NSLog(@"DisplayLink launched for screen with ID: %@", screenID);
+}
+
+- (void)handleDisplayLinkFired {
+    [_vsyncConditionLock lock];
+    [_vsyncConditionLock unlockWithCondition:1];
+}
+
+- (void)waitUntilVsync {
+    bool displayLinkOk = atomic_load(&_displayLinkOk);
+
+    /// If display link construction was corrupted, don't perform any waiting
+    if (!displayLinkOk) {
+        return;
+    }
+
+    [_vsyncConditionLock lockWhenCondition:1];
+    [_vsyncConditionLock unlockWithCondition:0];
+}
+
+- (void)waitForQueueSlot {
+    /// In case we receive more encoded command buffers, than gpu can handle (GPU bottleneck),
+    /// we need to throttle it down and start a new frame only when one currently run is finished
+    ///
+    /// see call place of `freeQueueSlot`
+    dispatch_semaphore_wait(_presentingBuffersExhaustionSemaphore, DISPATCH_TIME_FOREVER);
+}
+
+- (void)freeQueueSlot {
+    dispatch_semaphore_signal(_presentingBuffersExhaustionSemaphore);
+}
+
+- (void)invalidateDisplayLink {
+    if (_displayLink) {
+        CVDisplayLinkStop(_displayLink);
+        CVDisplayLinkRelease(_displayLink);
+        _displayLink = nil;
+    }
+}
+
+@end

skiko/src/awtMain/objectiveC/macos/MetalRedrawer.mm

@@ -3,11 +3,6 @@
 #import <jawt.h>
 #import <jawt_md.h>
 
-#import <Cocoa/Cocoa.h>
-#import <QuartzCore/QuartzCore.h>
-#import <Metal/Metal.h>
-#import <QuartzCore/CAMetalLayer.h>
-
 #import <GrBackendSurface.h>
 #import <GrDirectContext.h>
 #import <mtl/GrMtlBackendContext.h>
@@ -23,47 +18,11 @@
 #define AdpapterPriorityIntegrated 1
 #define AdpapterPriorityDiscrete 2
 
-@implementation AWTMetalLayer
-
-- (id)init
-{
-    self = [super init];
-
-    assert(self != NULL);
-
-    [self removeAllAnimations];
-    [self setAutoresizingMask: (kCALayerWidthSizable|kCALayerHeightSizable)];
-    [self setNeedsDisplayOnBoundsChange: YES];
-
-    return self;
-}
-
-@end
-
-@implementation MetalDevice
-
-- (id) init
-{
-    self = [super init];
-
-    if (self)
-    {
-        self.layer = nil;
-        self.adapter = nil;
-        self.queue = nil;
-        self.drawableHandle = nil;
-    }
-
-    return self;
-}
-
-@end
-
 extern "C"
 {
 
-extern "C" void* objc_autoreleasePoolPush(void);
-extern "C" void objc_autoreleasePoolPop(void*);
+void *objc_autoreleasePoolPush(void);
+void objc_autoreleasePoolPop(void *);
 
 JNIEXPORT jlong JNICALL Java_org_jetbrains_skiko_redrawer_MetalRedrawer_startRendering(
     JNIEnv * env, jobject redrawer)
@@ -72,9 +31,9 @@ JNIEXPORT jlong JNICALL Java_org_jetbrains_skiko_redrawer_MetalRedrawer_startRen
 }
 
 JNIEXPORT void JNICALL Java_org_jetbrains_skiko_redrawer_MetalRedrawer_endRendering(
-    JNIEnv * env, jobject redrawer, jlong handle)
+    JNIEnv *env, jobject redrawer, jlong handle)
 {
-    objc_autoreleasePoolPop((void*)handle);
+    objc_autoreleasePoolPop((void *)handle);
 }
 
 BOOL isUsingIntegratedGPU() {
@@ -148,38 +107,17 @@ JNIEXPORT jlong JNICALL Java_org_jetbrains_skiko_redrawer_MetalRedrawer_createMe
 {
     @autoreleasepool {
         id<MTLDevice> adapter = (__bridge_transfer id<MTLDevice>) (void *) adapterPtr;
-
-        MetalDevice *device = [MetalDevice new];
-
         NSObject<JAWT_SurfaceLayers>* dsi_mac = (__bridge NSObject<JAWT_SurfaceLayers> *) (void*) platformInfoPtr;
+        NSWindow *window = (__bridge NSWindow *) (void *) windowPtr;
 
         CALayer *container = [dsi_mac windowLayer];
-        [container removeAllAnimations];
-        [container setAutoresizingMask: (kCALayerWidthSizable|kCALayerHeightSizable)];
-        [container setNeedsDisplayOnBoundsChange: YES];
-
-        AWTMetalLayer *layer = [AWTMetalLayer new];
-        [container addSublayer: layer];
-        layer.javaRef = env->NewGlobalRef(redrawer);
 
-        id<MTLCommandQueue> fQueue = [adapter newCommandQueue];
+        MetalDevice *device = [[MetalDevice alloc] initWithContainer:container adapter:adapter window:window];
 
-        device.container = container;
-        device.layer = layer;
-        device.adapter = adapter;
-        device.queue = fQueue;
-
-        device.layer.device = device.adapter;
-        device.layer.pixelFormat = MTLPixelFormatBGRA8Unorm;
-        device.layer.contentsGravity = kCAGravityTopLeft;
-
-        CGFloat transparent[] = { 0.0f, 0.0f, 0.0f, 0.0f };
-        device.layer.backgroundColor = CGColorCreate(CGColorSpaceCreateDeviceRGB(), transparent);
-        device.layer.opaque = NO;
+        device.layer.javaRef = env->NewGlobalRef(redrawer);
 
         if (transparency)
         {
-            NSWindow* window = (__bridge NSWindow*) (void *) windowPtr;
             window.hasShadow = NO;
         }
 
@@ -282,10 +220,11 @@ JNIEXPORT jlong JNICALL Java_org_jetbrains_skiko_redrawer_MetalRedrawer_getAdapt
 }
 
 JNIEXPORT jboolean JNICALL Java_org_jetbrains_skiko_redrawer_MetalRedrawer_isOccluded(
-    JNIEnv *env, jobject redrawer, jlong windowPtr) {
+    JNIEnv *env, jobject redrawer, jlong devicePtr) {
     @autoreleasepool {
-        NSWindow* window = (__bridge NSWindow*) (void *) windowPtr;
-        return ([window occlusionState] & NSWindowOcclusionStateVisible) == 0;
+        MetalDevice *device = (__bridge MetalDevice *)(void *)devicePtr;
+
+        return ([device.window occlusionState] & NSWindowOcclusionStateVisible) == 0;
     }
 }
 

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

@@ -2,6 +2,7 @@ package org.jetbrains.skiko
 
 import kotlinx.coroutines.CompletableDeferred
 import kotlinx.coroutines.delay
+import kotlinx.coroutines.launch
 import kotlinx.coroutines.yield
 import org.jetbrains.skia.Canvas
 import org.jetbrains.skia.FontMgr
@@ -12,6 +13,7 @@ import org.jetbrains.skia.paragraph.ParagraphBuilder
 import org.jetbrains.skia.paragraph.ParagraphStyle
 import org.jetbrains.skia.paragraph.TextStyle
 import org.jetbrains.skiko.context.JvmContextHandler
+import org.jetbrains.skiko.redrawer.MetalRedrawer
 import org.jetbrains.skiko.redrawer.Redrawer
 import org.jetbrains.skiko.util.ScreenshotTestRule
 import org.jetbrains.skiko.util.UiTestScope
@@ -21,12 +23,11 @@ import org.junit.Assert.assertEquals
 import org.junit.Assume.assumeTrue
 import org.junit.Rule
 import org.junit.Test
+import org.junit.Ignore
 import java.awt.BorderLayout
 import java.awt.Color
 import java.awt.Dimension
-import java.awt.event.ComponentAdapter
-import java.awt.event.ComponentEvent
-import java.awt.event.WindowEvent
+import java.awt.event.*
 import javax.swing.JFrame
 import javax.swing.JLayeredPane
 import javax.swing.JPanel
@@ -35,6 +36,8 @@ import javax.swing.WindowConstants
 import kotlin.random.Random
 import kotlin.test.assertNotNull
 import kotlin.test.assertTrue
+import kotlin.time.Duration
+import kotlin.time.ExperimentalTime
 
 @Suppress("BlockingMethodInNonBlockingContext", "SameParameterValue")
 class SkiaLayerTest {
@@ -92,6 +95,69 @@ class SkiaLayerTest {
         )
     }
 
+    @OptIn(ExperimentalTime::class)
+    @Ignore
+    @Test
+    fun `frame is rendered immediately`() = uiTest {
+        val window = UiTestWindow(
+            properties = SkiaLayerProperties(
+                isVsyncEnabled = true
+            )
+        )
+        val colors = arrayOf(
+            Color.RED,
+            Color.GREEN,
+            Color.BLUE,
+            Color.YELLOW
+        )
+
+        var counter1 = 0
+        var counter2 = 0
+        val paint = Paint()
+
+        try {
+            window.setLocation(200, 200)
+            window.setSize(400, 600)
+            window.defaultCloseOperation = WindowConstants.EXIT_ON_CLOSE
+            window.layer.skikoView = object : SkikoView {
+                override fun onRender(canvas: Canvas, width: Int, height: Int, nanoTime: Long) {
+                    val c1 = counter1
+                    val c2 = counter2
+
+                    paint.color = colors[c1.mod(colors.size)].rgb
+                    canvas.drawRect(Rect(0f, 0f, width.toFloat(), height / 2f), paint)
+
+                    paint.color = colors[c2.mod(colors.size)].rgb
+                    canvas.drawRect(Rect(0f, height / 2f, width.toFloat(), height.toFloat()), paint)
+                }
+            }
+            window.isVisible = true
+
+            window.addKeyListener(object : KeyAdapter() {
+                override fun keyTyped(e: KeyEvent?) {
+                    launch {
+                        val redrawer = window.layer.redrawer as MetalRedrawer
+                        redrawer.drawSync()
+                        counter1 += 1
+                        redrawer.drawSync()
+                        counter2 += 1
+                        redrawer.drawSync()
+                    }
+                }
+            });
+
+            window.addWindowListener(object : WindowAdapter() {
+                override fun windowActivated(e: WindowEvent?) {
+                    window.requestFocus()
+                }
+            })
+
+            delay(Duration.INFINITE)
+        } finally {
+            window.close()
+        }
+    }
+
     @Test
     fun `render single window`() = uiTest {
         val window = UiTestWindow()

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

@@ -10,9 +10,10 @@ import javax.swing.JFrame
 
 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 renderApi = System.getProperty("skiko.test.ui.renderApi", "all")
+//    val renderApi = System.getProperty("skiko.test.ui.renderApi", "all")
+    val renderApi = "metal"
 
     runBlocking(MainUIDispatcher) {
         if (renderApi == "all") {

skiko/src/commonMain/kotlin/org/jetbrains/skiko/FrameDispatcher.kt

@@ -13,11 +13,11 @@ import kotlin.coroutines.CoroutineContext
  */
 class FrameDispatcher(
     scope: CoroutineScope,
-    private val onFrame: suspend () -> Unit
+    private val onFrame: suspend () -> Unit,
 ) {
     constructor(
         context: CoroutineContext,
-        onFrame: suspend () -> Unit
+        onFrame: suspend () -> Unit,
     ) : this(
         CoroutineScope(context),
         onFrame
@@ -28,7 +28,9 @@ class FrameDispatcher(
 
     private val job = scope.launch {
         while (true) {
+            // Await for draw request
             frameChannel.receive()
+
             frameScheduled = false
             onFrame()
             // As per `yield()` documentation: