Skip to content

Commit

Permalink
In MetalRedrawer, stop delaying rendering as soon as the window becom…
Browse files Browse the repository at this point in the history
…es non-occluded. (#822)

In MetalRedrawer, stop delaying rendering as soon as the window becomes non-occluded.
  • Loading branch information
m-sasha authored Nov 8, 2023
1 parent 5825d5c commit b2149d9
Show file tree
Hide file tree
Showing 7 changed files with 128 additions and 30 deletions.
1 change: 1 addition & 0 deletions skiko/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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.*

/**
Expand Down Expand Up @@ -62,6 +62,9 @@ internal class MetalRedrawer(
private val adapter = chooseMetalAdapter(properties.adapterPriority)
private val displayLinkThrottler = DisplayLinkThrottler()

private val windowOcclusionStateChannel = Channel<Boolean>(Channel.CONFLATED)
@Volatile private var isWindowOccluded = false

init {
onDeviceChosen(adapter.name)
val initDevice = layer.backedLayer.useDrawingSurfacePlatformInfo {
Expand All @@ -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())
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
}
1 change: 1 addition & 0 deletions skiko/src/awtMain/objectiveC/macos/MetalDevice.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
@property (strong) id<MTLCommandQueue> queue;
@property (strong) id<CAMetalDrawable> drawableHandle;
@property (strong) dispatch_semaphore_t inflightSemaphore;
@property (strong) id<NSObject> occlusionObserver;

@end

Expand Down
56 changes: 45 additions & 11 deletions skiko/src/awtMain/objectiveC/macos/MetalRedrawer.mm
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@

#import "MetalDevice.h"

#include <assert.h>

#include "common/interop.hh"

@implementation AWTMetalLayer

- (id)init
Expand Down Expand Up @@ -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"
{

Expand Down Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -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
42 changes: 38 additions & 4 deletions skiko/src/awtTest/kotlin/org/jetbrains/skiko/SkiaLayerTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -59,7 +58,6 @@ class SkiaLayerTest {
@get:Rule
val screenshots = ScreenshotTestRule()

@OptIn(ExperimentalTime::class)
@Ignore
@Test
fun `metal drawables not lost`() = uiTest {
Expand Down Expand Up @@ -108,7 +106,7 @@ class SkiaLayerTest {
redrawer.drawSync()
}
}
});
})

window.addWindowListener(object : WindowAdapter() {
override fun windowActivated(e: WindowEvent?) {
Expand Down Expand Up @@ -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<Long>()
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)
Expand Down
27 changes: 20 additions & 7 deletions skiko/src/awtTest/kotlin/org/jetbrains/skiko/util/UiTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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<GraphicsApi> = 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()
}
}
}
}
Expand Down
5 changes: 5 additions & 0 deletions skiko/src/jvmMain/cpp/common/impl/Library.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<void**>(&env), SKIKO_JNI_VERSION) != JNI_OK)
return JNI_ERR;
Expand Down

0 comments on commit b2149d9

Please sign in to comment.