Unverified Commit 27e5d7ea authored by Igor Demin's avatar Igor Demin Committed by GitHub

Fix a native crash on `makeGL` (#869)

It is a regression after https://github.com/JetBrains/skiko/pull/811

After that PR, OpenGL functions are linked in runtime via function `loadOpenGLLibrary`. We didn't call it in `makeGL` functions. Now we call.

Adding also `isLoaded` to avoid loading it multiple times

Fixes https://youtrack.jetbrains.com/issue/COMPOSE-642/Fix-Skiko-API-regression-for-creating-OpenGL-context
parent 2a8e25d8
......@@ -54,7 +54,7 @@ extern "C" {
return wglGetCurrentContext();
}
JNIEXPORT void JNICALL Java_org_jetbrains_skiko_OpenGLLibraryKt_loadOpenGLLibraryWindows(JNIEnv *env, jobject obj) {
JNIEXPORT void JNICALL Java_org_jetbrains_skiko_OpenGLLibrary_1jvmKt_loadOpenGLLibraryWindows(JNIEnv *env, jobject obj) {
OpenGL32Library = LoadLibrary("opengl32.dll");
if (OpenGL32Library == nullptr) {
auto code = GetLastError();
......
package org.jetbrains.skiko
import org.jetbrains.skia.BackendRenderTarget
import org.jetbrains.skiko.util.joinThreadCatching
import kotlin.test.*
class BackendRenderTargetTest {
@Test
fun makeGL() {
// use a new thread to be sure that there is no OpenGL context in it
joinThreadCatching {
// this method doesn't fail, even if there is no OpenGL Context in the thread or there is no OpenGL support
BackendRenderTarget.makeGL(100, 100, 1, 8, 0, 0x8058)
}
}
}
\ No newline at end of file
package org.jetbrains.skiko
import org.jetbrains.skia.DirectContext
import org.jetbrains.skiko.util.joinThreadCatching
import kotlin.test.*
class DirectContextTest {
@Test
fun makeGL() {
// use a new thread to be sure that there is no OpenGL context in it
joinThreadCatching {
// Test only that the method doesn't crash VM, and only throw an exception.
// To properly test it, we have to use third-party created OpenGL context.
// Testing with our own created OpenGL context won't be fair, as these functions can do additional
// initialization (i.e. if we call our own glMakeCurrent, we have to call `loadOpenGLLibrary`)
var actualException: RenderException? = null
try {
DirectContext.makeGL()
} catch (e: RenderException) {
actualException = e
}
assertTrue(actualException != null, "makeGL didn't fail")
}
}
}
package org.jetbrains.skiko.util
/**
* Runs body in another thread, join it and throws a last exception in this thread if any
*/
fun joinThreadCatching(body: () -> Unit) {
var exception: Throwable? = null
val thread = object : Thread() {
override fun run() = body()
}
thread.setUncaughtExceptionHandler { _, e ->
exception = e
}
thread.start()
thread.join()
if (exception != null) {
throw exception!!
}
}
\ No newline at end of file
......@@ -4,6 +4,8 @@ import org.jetbrains.skia.impl.Library.Companion.staticLoad
import org.jetbrains.skia.impl.Managed
import org.jetbrains.skia.impl.NativePointer
import org.jetbrains.skia.impl.Stats
import org.jetbrains.skiko.RenderException
import org.jetbrains.skiko.loadOpenGLLibrary
class BackendRenderTarget internal constructor(ptr: NativePointer) : Managed(ptr, _FinalizerHolder.PTR) {
companion object {
......@@ -16,16 +18,9 @@ class BackendRenderTarget internal constructor(ptr: NativePointer) : Managed(ptr
fbFormat: Int
): BackendRenderTarget {
Stats.onNativeCall()
return BackendRenderTarget(
_nMakeGL(
width,
height,
sampleCnt,
stencilBits,
fbId,
fbFormat
)
)
val ptr =_nMakeGL(width, height, sampleCnt, stencilBits, fbId, fbFormat)
if (ptr == NullPointer) throw RenderException("Can't create OpenGL BackendRenderTarget")
return BackendRenderTarget(ptr)
}
fun makeMetal(width: Int, height: Int, texturePtr: NativePointer): BackendRenderTarget {
......
......@@ -2,12 +2,17 @@ package org.jetbrains.skia
import org.jetbrains.skia.impl.*
import org.jetbrains.skia.impl.Library.Companion.staticLoad
import org.jetbrains.skiko.RenderException
import org.jetbrains.skiko.loadOpenGLLibrary
class DirectContext internal constructor(ptr: NativePointer) : RefCnt(ptr) {
companion object {
fun makeGL(): DirectContext {
Stats.onNativeCall()
return DirectContext(_nMakeGL())
loadOpenGLLibrary()
val ptr = _nMakeGL()
if (ptr == NullPointer) throw RenderException("Can't create OpenGL DirectContext")
return DirectContext(ptr)
}
fun makeMetal(devicePtr: NativePointer, queuePtr: NativePointer): DirectContext {
......
......@@ -5,19 +5,10 @@ package org.jetbrains.skiko
*
* Should be called before any OpenGL operation.
*
* The current implementation loads OpenGl lazily on Windows, and at app startup on Linux/macOs.
* The current implementation loads OpenGl lazily on Windows, and at app startup on Linux/macOs, native and web targets
*
* Some Windows machines don't have OpenGL (usually CI machines), so we'll fallback to other renderers on them.
*
* Throws [RenderException] if OpenGL library can't be loaded.
* @throws RenderException if OpenGL library can't be loaded.
*/
internal fun loadOpenGLLibrary() {
if (hostOs.isWindows) {
loadOpenGLLibraryWindows()
} else {
// do nothing, the library should be already available
}
}
@Synchronized
private external fun loadOpenGLLibraryWindows()
\ No newline at end of file
internal expect fun loadOpenGLLibrary()
\ No newline at end of file
package org.jetbrains.skiko
import org.jetbrains.skia.impl.Library
private external fun loadOpenGLLibraryWindows()
private var isLoaded = false
@Synchronized
internal actual fun loadOpenGLLibrary() {
if (!isLoaded) {
when {
// On Windows it is linked dynamically
hostOs.isWindows -> {
Library.staticLoad()
loadOpenGLLibraryWindows()
}
// it was deprecated in macOS 10.14 and isn't supported in Skiko
// see https://developer.apple.com/library/archive/documentation/GraphicsImaging/Conceptual/OpenGL-MacProgGuide/opengl_intro/opengl_intro.html
hostOs.isMacOS -> throw RenderException("OpenGL on macOS isn't supported")
// Do nothing as we don't know for sure which OS supports OpenGL.
// If there is support, we assume that it is already linked.
// If there is no support, there will be a native crash
// (to throw a RenderException we need to investigate case by case)
else -> Unit
}
isLoaded = true
}
}
package org.jetbrains.skiko
internal actual fun loadOpenGLLibrary() {
when (hostOs) {
// It is deprecated. See https://developer.apple.com/documentation/opengles
OS.Ios -> throw RenderException("OpenGL on iOS isn't supported")
// Do nothing as we don't know for sure which OS supports OpenGL.
// If there is support, we assume that it is already linked.
// If there is no support, there will be a native crash
// (to throw a RenderException we need to investigate case by case)
else -> Unit
}
}
\ No newline at end of file
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