From b2149d9819c1763c5b1ac5088c16d601d28d77bc Mon Sep 17 00:00:00 2001 From: Alexander Maryanovsky Date: Wed, 8 Nov 2023 20:05:59 +0200 Subject: [PATCH] In MetalRedrawer, stop delaying rendering as soon as the window becomes non-occluded. (#822) In MetalRedrawer, stop delaying rendering as soon as the window becomes non-occluded. --- skiko/build.gradle.kts | 1 + .../jetbrains/skiko/redrawer/MetalRedrawer.kt | 26 ++++++--- .../awtMain/objectiveC/macos/MetalDevice.h | 1 + .../awtMain/objectiveC/macos/MetalRedrawer.mm | 56 +++++++++++++++---- .../org/jetbrains/skiko/SkiaLayerTest.kt | 42 ++++++++++++-- .../kotlin/org/jetbrains/skiko/util/UiTest.kt | 27 ++++++--- skiko/src/jvmMain/cpp/common/impl/Library.cc | 5 ++ 7 files changed, 128 insertions(+), 30 deletions(-) diff --git a/skiko/build.gradle.kts b/skiko/build.gradle.kts index 66035f4dd..1a6c6bfe9 100644 --- a/skiko/build.gradle.kts +++ b/skiko/build.gradle.kts @@ -765,6 +765,7 @@ fun createObjcCompileTask( includeHeadersNonRecursive(skiaHeadersDirs(skiaJvmBindingsDir.get())) includeHeadersNonRecursive(projectDir.resolve("src/awtMain/cpp/include")) includeHeadersNonRecursive(projectDir.resolve("src/commonMain/cpp/common/include")) + includeHeadersNonRecursive(projectDir.resolve("src/jvmMain/cpp")) compiler.set("clang") buildVariant.set(buildType) diff --git a/skiko/src/awtMain/kotlin/org/jetbrains/skiko/redrawer/MetalRedrawer.kt b/skiko/src/awtMain/kotlin/org/jetbrains/skiko/redrawer/MetalRedrawer.kt index 62a8ef020..9812053f5 100644 --- a/skiko/src/awtMain/kotlin/org/jetbrains/skiko/redrawer/MetalRedrawer.kt +++ b/skiko/src/awtMain/kotlin/org/jetbrains/skiko/redrawer/MetalRedrawer.kt @@ -1,9 +1,9 @@ package org.jetbrains.skiko.redrawer import kotlinx.coroutines.* +import kotlinx.coroutines.channels.Channel import org.jetbrains.skiko.* import org.jetbrains.skiko.context.MetalContextHandler -import java.util.concurrent.Executors import javax.swing.SwingUtilities.* /** @@ -62,6 +62,9 @@ internal class MetalRedrawer( private val adapter = chooseMetalAdapter(properties.adapterPriority) private val displayLinkThrottler = DisplayLinkThrottler() + private val windowOcclusionStateChannel = Channel(Channel.CONFLATED) + @Volatile private var isWindowOccluded = false + init { onDeviceChosen(adapter.name) val initDevice = layer.backedLayer.useDrawingSurfacePlatformInfo { @@ -74,8 +77,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()) @@ -137,10 +138,20 @@ internal class MetalRedrawer( if (isDisposed) throw CancellationException() // 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)) - delay(300) + if (isWindowOccluded) { + withTimeoutOrNull(300) { + // If the window becomes non-occluded, stop waiting immediately + @Suppress("ControlFlowWithEmptyBody") + while (windowOcclusionStateChannel.receive()) { } + } + } + } + + // Called from MetalRedrawer.mm + @Suppress("unused") + fun onOcclusionStateChanged(isOccluded: Boolean) { + isWindowOccluded = isOccluded + windowOcclusionStateChannel.trySend(isOccluded) } private fun performDraw() = synchronized(drawLock) { @@ -180,5 +191,4 @@ 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 } diff --git a/skiko/src/awtMain/objectiveC/macos/MetalDevice.h b/skiko/src/awtMain/objectiveC/macos/MetalDevice.h index b6c591927..b9514781c 100644 --- a/skiko/src/awtMain/objectiveC/macos/MetalDevice.h +++ b/skiko/src/awtMain/objectiveC/macos/MetalDevice.h @@ -14,6 +14,7 @@ @property (strong) id queue; @property (strong) id drawableHandle; @property (strong) dispatch_semaphore_t inflightSemaphore; +@property (strong) id occlusionObserver; @end diff --git a/skiko/src/awtMain/objectiveC/macos/MetalRedrawer.mm b/skiko/src/awtMain/objectiveC/macos/MetalRedrawer.mm index edf0230e9..ef9edd34c 100644 --- a/skiko/src/awtMain/objectiveC/macos/MetalRedrawer.mm +++ b/skiko/src/awtMain/objectiveC/macos/MetalRedrawer.mm @@ -15,6 +15,10 @@ #import "MetalDevice.h" +#include + +#include "common/interop.hh" + @implementation AWTMetalLayer - (id)init @@ -51,6 +55,32 @@ - (id) init @end +/// Linked from skiko/src/jvmMain/cpp/common/impl/Library.cc +/// clang treats extern symbol declarations as C in Objective-C++(.mm) and doesn't mangle them +extern JavaVM *jvm; + +static JNIEnv *resolveJNIEnvForCurrentThread() { + JNIEnv *env; + int envStat = jvm->GetEnv((void **)&env, SKIKO_JNI_VERSION); + + if (envStat == JNI_EDETACHED) { + jvm->AttachCurrentThread((void **) &env, NULL); + } + + assert(env); + + return env; +} + +static jmethodID getOnOcclusionStateChangedMethodID(JNIEnv *env, jobject redrawer) { + static jmethodID onOcclusionStateChanged = NULL; + if (onOcclusionStateChanged == NULL) { + jclass redrawerClass = env->GetObjectClass(redrawer); + onOcclusionStateChanged = env->GetMethodID(redrawerClass, "onOcclusionStateChanged", "(Z)V"); + } + return onOcclusionStateChanged; +} + extern "C" { @@ -92,12 +122,23 @@ JNIEXPORT jlong JNICALL Java_org_jetbrains_skiko_redrawer_MetalRedrawer_createMe /// max inflight command buffers count matches swapchain size to avoid overcommitment device.inflightSemaphore = dispatch_semaphore_create(device.layer.maximumDrawableCount); - if (transparency) - { - NSWindow* window = (__bridge NSWindow*) (void *) windowPtr; + NSWindow* window = (__bridge NSWindow*) (void *) windowPtr; + + if (transparency) { window.hasShadow = NO; } + jmethodID onOcclusionStateChanged = getOnOcclusionStateChangedMethodID(env, redrawer); + device.occlusionObserver = + [[NSNotificationCenter defaultCenter] addObserverForName:NSWindowDidChangeOcclusionStateNotification + object:window + queue:[NSOperationQueue mainQueue] + usingBlock:^(NSNotification * _Nonnull note) { + BOOL isOccluded = ([window occlusionState] & NSWindowOcclusionStateVisible) == 0; + JNIEnv *jniEnv = resolveJNIEnvForCurrentThread(); + jniEnv->CallObjectMethod(layer.javaRef, onOcclusionStateChanged, isOccluded); + }]; + return (jlong) (__bridge_retained void *) device; } } @@ -171,18 +212,11 @@ JNIEXPORT void JNICALL Java_org_jetbrains_skiko_redrawer_MetalRedrawer_disposeDe @autoreleasepool { MetalDevice *device = (__bridge_transfer MetalDevice *) (void *) devicePtr; env->DeleteGlobalRef(device.layer.javaRef); + [[NSNotificationCenter defaultCenter] removeObserver:device.occlusionObserver]; [device.layer removeFromSuperlayer]; [CATransaction flush]; } } -JNIEXPORT jboolean JNICALL Java_org_jetbrains_skiko_redrawer_MetalRedrawer_isOccluded( - JNIEnv *env, jobject redrawer, jlong windowPtr) { - @autoreleasepool { - NSWindow* window = (__bridge NSWindow*) (void *) windowPtr; - return ([window occlusionState] & NSWindowOcclusionStateVisible) == 0; - } -} - } // extern C #endif diff --git a/skiko/src/awtTest/kotlin/org/jetbrains/skiko/SkiaLayerTest.kt b/skiko/src/awtTest/kotlin/org/jetbrains/skiko/SkiaLayerTest.kt index b3ae447f0..f0b9eaf42 100644 --- a/skiko/src/awtTest/kotlin/org/jetbrains/skiko/SkiaLayerTest.kt +++ b/skiko/src/awtTest/kotlin/org/jetbrains/skiko/SkiaLayerTest.kt @@ -37,9 +37,8 @@ import kotlin.random.Random import kotlin.test.assertNotNull import kotlin.test.assertTrue import kotlin.time.Duration -import kotlin.time.ExperimentalTime -@Suppress("BlockingMethodInNonBlockingContext", "SameParameterValue") +@Suppress("SameParameterValue") class SkiaLayerTest { private val fontCollection = FontCollection() .setDefaultFontManager(FontMgr.default) @@ -59,7 +58,6 @@ class SkiaLayerTest { @get:Rule val screenshots = ScreenshotTestRule() - @OptIn(ExperimentalTime::class) @Ignore @Test fun `metal drawables not lost`() = uiTest { @@ -108,7 +106,7 @@ class SkiaLayerTest { redrawer.drawSync() } } - }); + }) window.addWindowListener(object : WindowAdapter() { override fun windowActivated(e: WindowEvent?) { @@ -689,6 +687,42 @@ class SkiaLayerTest { } } + @Test + fun `second frame drawn without delay in metal`() = uiTest( + // SOFTWARE_COMPAT fails because it's just too slow + excludeRenderApis = listOf(GraphicsApi.SOFTWARE_COMPAT) + ) { + val renderTimes = mutableListOf() + val renderer = object: SkikoView { + override fun onRender(canvas: Canvas, width: Int, height: Int, nanoTime: Long) { + renderTimes.add(System.currentTimeMillis()) + } + } + val window = UiTestWindow { + layer.skikoView = renderer + contentPane.add(layer, BorderLayout.CENTER) + } + try { + window.size = Dimension(800, 800) + repeat(10) { + window.isVisible = true + delay(16) + window.layer.needRedraw() + delay(500) + window.isVisible = false + + val dt = renderTimes.last() - renderTimes.first() + assertTrue( + actual = dt < 100, + message = "2nd frame drawn ${dt}ms after 1st" + ) + renderTimes.clear() + } + } finally { + window.dispose() + } + } + @Test fun `render text (Windows)`() { testRenderText(OS.Windows) diff --git a/skiko/src/awtTest/kotlin/org/jetbrains/skiko/util/UiTest.kt b/skiko/src/awtTest/kotlin/org/jetbrains/skiko/util/UiTest.kt index 8e9bf258b..5a6f4b464 100644 --- a/skiko/src/awtTest/kotlin/org/jetbrains/skiko/util/UiTest.kt +++ b/skiko/src/awtTest/kotlin/org/jetbrains/skiko/util/UiTest.kt @@ -8,21 +8,34 @@ import org.junit.Assume.assumeTrue import java.awt.GraphicsEnvironment import javax.swing.JFrame -internal fun uiTest(block: suspend UiTestScope.() -> Unit) { +internal fun uiTest( + excludeRenderApis: List = emptyList(), + block: suspend UiTestScope.() -> Unit +) { assumeFalse(GraphicsEnvironment.isHeadless()) assumeTrue(System.getProperty("skiko.test.ui.enabled", "false") == "true") - val renderApi = System.getProperty("skiko.test.ui.renderApi", "all") + val renderApiProperty = System.getProperty("skiko.test.ui.renderApi", "all") runBlocking(MainUIDispatcher) { - if (renderApi == "all") { - SkikoProperties.fallbackRenderApiQueue(SkikoProperties.renderApi).forEach { - println("Testing $it renderApi") + if (renderApiProperty == "all") { + for (renderApi in SkikoProperties.fallbackRenderApiQueue(SkikoProperties.renderApi)) { + if (renderApi in excludeRenderApis) { + println("Skipping $renderApi renderApi") + continue + } + println("Testing $renderApi renderApi") println() - UiTestScope(scope = this, renderApi = it).block() + UiTestScope(scope = this, renderApi = renderApi).block() } } else { - UiTestScope(scope = this, renderApi = SkikoProperties.parseRenderApi(renderApi)).block() + val renderApi = SkikoProperties.parseRenderApi(renderApiProperty) + if (renderApi in excludeRenderApis) { + println("Skipping $renderApi renderApi") + } + else { + UiTestScope(scope = this, renderApi = renderApi).block() + } } } } diff --git a/skiko/src/jvmMain/cpp/common/impl/Library.cc b/skiko/src/jvmMain/cpp/common/impl/Library.cc index 3e254ce8a..53f81b139 100644 --- a/skiko/src/jvmMain/cpp/common/impl/Library.cc +++ b/skiko/src/jvmMain/cpp/common/impl/Library.cc @@ -5,7 +5,12 @@ #include "../paragraph/interop.hh" #include "../svg/interop.hh" + +extern "C" JavaVM *jvm = NULL; + JNIEXPORT jint JNICALL JNI_OnLoad(JavaVM* vm, void* reserved) { + jvm = vm; + JNIEnv* env; if (vm->GetEnv(reinterpret_cast(&env), SKIKO_JNI_VERSION) != JNI_OK) return JNI_ERR;