Skip to content

Commit 569acc9

Browse files
authored
fix(🤖): fix minor threading issue on Android (#2725)
The main goal of this PR is to refactor the platforms-specific Skia code in a way that will make the Graphite migration smoother. It also fixes a minor issue on Android where one onscreen contextes could created in many threads instead of only once. This issue is most likely not affecting any users.
1 parent ce475ac commit 569acc9

23 files changed

+318
-453
lines changed

Diff for: ‎apps/paper/src/Examples/WebGPU/utils.ts

+10-7
Original file line numberDiff line numberDiff line change
@@ -7,26 +7,26 @@ import {
77
Skia,
88
} from "@shopify/react-native-skia";
99
import { Dimensions, PixelRatio } from "react-native";
10-
import { useSharedValue } from "react-native-reanimated";
10+
import { runOnUI, useSharedValue } from "react-native-reanimated";
1111
import { useCanvasEffect } from "react-native-wgpu";
1212

13+
const pd = PixelRatio.get();
14+
1315
export const useSkiaContext = () => {
1416
const context = useSharedValue<SkiaContext | null>(null);
1517
const ref = useCanvasEffect(() => {
1618
const nativeSurface = ref.current!.getNativeSurface();
17-
context.value = Skia.Context(
18-
nativeSurface.surface,
19-
nativeSurface.width * pd,
20-
nativeSurface.height * pd
21-
);
19+
const { surface, width, height } = nativeSurface;
20+
runOnUI(() => {
21+
context.value = Skia.Context(surface, width * pd, height * pd);
22+
})();
2223
});
2324
return {
2425
context,
2526
ref,
2627
};
2728
};
2829

29-
const pd = PixelRatio.get();
3030
const { width, height } = Dimensions.get("window");
3131
const center = { x: width / 2, y: height / 2 };
3232
const R = width / 4;
@@ -44,6 +44,9 @@ p2.setColor(Skia.Color(c2));
4444
export const drawBreatheDemo = (ctx: SkiaContext, progress: number) => {
4545
"worklet";
4646
const surface = ctx.getSurface();
47+
if (surface === null) {
48+
throw new Error("No surface available");
49+
}
4750
const canvas = surface.getCanvas();
4851
canvas.clear(Skia.Color("rgb(36, 43, 56)"));
4952
canvas.save();
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
#pragma once
2+
3+
#include "SkiaOpenGLSurfaceFactory.h"
4+
#include "WindowContext.h"
5+
6+
#include "include/core/SkSurface.h"
7+
8+
class OpenGLContext {
9+
public:
10+
OpenGLContext(const OpenGLContext &) = delete;
11+
OpenGLContext &operator=(const OpenGLContext &) = delete;
12+
13+
static OpenGLContext &getInstance() {
14+
static thread_local OpenGLContext instance;
15+
return instance;
16+
}
17+
18+
sk_sp<SkSurface> MakeOffscreen(int width, int height) {
19+
return RNSkia::SkiaOpenGLSurfaceFactory::makeOffscreenSurface(
20+
&_context, width, height);
21+
}
22+
23+
sk_sp<SkImage> MakeImageFromBuffer(void *buffer) {
24+
return RNSkia::SkiaOpenGLSurfaceFactory::makeImageFromHardwareBuffer(
25+
&_context, buffer);
26+
}
27+
28+
std::unique_ptr<RNSkia::WindowContext> MakeWindow(ANativeWindow *window,
29+
int width, int height) {
30+
return RNSkia::SkiaOpenGLSurfaceFactory::makeContext(&_context, window,
31+
width, height);
32+
}
33+
34+
private:
35+
RNSkia::SkiaOpenGLContext _context;
36+
37+
OpenGLContext() {
38+
RNSkia::SkiaOpenGLHelper::createSkiaDirectContextIfNecessary(&_context);
39+
}
40+
};

Diff for: ‎packages/skia/android/cpp/rnskia-android/RNSkAndroidPlatformContext.h

+5-5
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,9 @@
1111

1212
#include "AHardwareBufferUtils.h"
1313
#include "JniPlatformContext.h"
14+
#include "OpenGLContext.h"
1415
#include "RNSkAndroidVideo.h"
1516
#include "RNSkPlatformContext.h"
16-
#include "SkiaOpenGLSurfaceFactory.h"
1717

1818
#pragma clang diagnostic push
1919
#pragma clang diagnostic ignored "-Wdocumentation"
@@ -51,17 +51,17 @@ class RNSkAndroidPlatformContext : public RNSkPlatformContext {
5151
}
5252

5353
sk_sp<SkSurface> makeOffscreenSurface(int width, int height) override {
54-
return SkiaOpenGLSurfaceFactory::makeOffscreenSurface(width, height);
54+
return OpenGLContext::getInstance().MakeOffscreen(width, height);
5555
}
5656

57-
std::shared_ptr<SkiaContext>
57+
std::shared_ptr<WindowContext>
5858
makeContextFromNativeSurface(void *surface, int width, int height) override {
59-
return SkiaOpenGLSurfaceFactory::makeContext(
59+
return OpenGLContext::getInstance().MakeWindow(
6060
reinterpret_cast<ANativeWindow *>(surface), width, height);
6161
}
6262

6363
sk_sp<SkImage> makeImageFromNativeBuffer(void *buffer) override {
64-
return SkiaOpenGLSurfaceFactory::makeImageFromHardwareBuffer(buffer);
64+
return OpenGLContext::getInstance().MakeImageFromBuffer(buffer);
6565
}
6666

6767
std::shared_ptr<RNSkVideo> createVideo(const std::string &url) override {

Diff for: ‎packages/skia/android/cpp/rnskia-android/RNSkAndroidVideo.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@
1313

1414
#pragma clang diagnostic pop
1515

16+
#include "OpenGLContext.h"
1617
#include "RNSkAndroidVideo.h"
17-
#include "SkiaOpenGLSurfaceFactory.h"
1818

1919
namespace RNSkia {
2020

@@ -52,7 +52,7 @@ sk_sp<SkImage> RNSkAndroidVideo::nextImage(double *timeStamp) {
5252
// Convert jobject to AHardwareBuffer
5353
AHardwareBuffer *buffer =
5454
AHardwareBuffer_fromHardwareBuffer(env, jHardwareBuffer);
55-
return SkiaOpenGLSurfaceFactory::makeImageFromHardwareBuffer(buffer);
55+
return OpenGLContext::getInstance().MakeImageFromBuffer(buffer);
5656
#else
5757
return nullptr;
5858
#endif

Diff for: ‎packages/skia/android/cpp/rnskia-android/RNSkOpenGLCanvasProvider.cpp

+49-14
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
#include <memory>
44

5+
#include "OpenGLContext.h"
6+
57
#pragma clang diagnostic push
68
#pragma clang diagnostic ignored "-Wdocumentation"
79

@@ -20,33 +22,41 @@ RNSkOpenGLCanvasProvider::RNSkOpenGLCanvasProvider(
2022
RNSkOpenGLCanvasProvider::~RNSkOpenGLCanvasProvider() {}
2123

2224
float RNSkOpenGLCanvasProvider::getScaledWidth() {
23-
return _surfaceHolder ? _surfaceHolder->getWidth() : 0;
25+
if (_surfaceHolder) {
26+
return static_cast<float>(_surfaceHolder->getWidth());
27+
}
28+
return 0;
2429
}
2530

2631
float RNSkOpenGLCanvasProvider::getScaledHeight() {
27-
return _surfaceHolder ? _surfaceHolder->getHeight() : 0;
32+
if (_surfaceHolder) {
33+
return static_cast<float>(_surfaceHolder->getHeight());
34+
}
35+
return 0;
2836
}
2937

3038
bool RNSkOpenGLCanvasProvider::renderToCanvas(
3139
const std::function<void(SkCanvas *)> &cb) {
32-
40+
JNIEnv *env = facebook::jni::Environment::current();
3341
if (_surfaceHolder != nullptr && cb != nullptr) {
3442
// Get the surface
3543
auto surface = _surfaceHolder->getSurface();
36-
if (surface) {
37-
38-
// Ensure we are ready to render
39-
if (!_surfaceHolder->makeCurrent()) {
40-
return false;
41-
}
42-
_surfaceHolder->updateTexImage();
44+
env->CallVoidMethod(_jSurfaceTexture, _updateTexImageMethod);
4345

46+
// Check for exceptions
47+
if (env->ExceptionCheck()) {
48+
RNSkLogger::logToConsole("updateAndRelease() failed. The exception above "
49+
"can safely be ignored");
50+
env->ExceptionClear();
51+
}
52+
if (surface) {
4453
// Draw into canvas using callback
4554
cb(surface->getCanvas());
4655

4756
// Swap buffers and show on screen
48-
return _surfaceHolder->present();
57+
_surfaceHolder->present();
4958

59+
return true;
5060
} else {
5161
// the render context did not provide a surface
5262
return false;
@@ -56,11 +66,31 @@ bool RNSkOpenGLCanvasProvider::renderToCanvas(
5666
return false;
5767
}
5868

59-
void RNSkOpenGLCanvasProvider::surfaceAvailable(jobject surface, int width,
60-
int height) {
69+
void RNSkOpenGLCanvasProvider::surfaceAvailable(jobject jSurfaceTexture,
70+
int width, int height) {
6171
// Create renderer!
72+
JNIEnv *env = facebook::jni::Environment::current();
73+
74+
_jSurfaceTexture = env->NewGlobalRef(jSurfaceTexture);
75+
jclass surfaceClass = env->FindClass("android/view/Surface");
76+
jmethodID surfaceConstructor = env->GetMethodID(
77+
surfaceClass, "<init>", "(Landroid/graphics/SurfaceTexture;)V");
78+
// Create a new Surface instance
79+
jobject jSurface =
80+
env->NewObject(surfaceClass, surfaceConstructor, jSurfaceTexture);
81+
82+
jclass surfaceTextureClass = env->GetObjectClass(_jSurfaceTexture);
83+
_updateTexImageMethod =
84+
env->GetMethodID(surfaceTextureClass, "updateTexImage", "()V");
85+
86+
// Acquire the native window from the Surface
87+
auto window = ANativeWindow_fromSurface(env, jSurface);
88+
// Clean up local references
89+
env->DeleteLocalRef(jSurface);
90+
env->DeleteLocalRef(surfaceClass);
91+
env->DeleteLocalRef(surfaceTextureClass);
6292
_surfaceHolder =
63-
SkiaOpenGLSurfaceFactory::makeWindowedSurface(surface, width, height);
93+
OpenGLContext::getInstance().MakeWindow(window, width, height);
6494

6595
// Post redraw request to ensure we paint in the next draw cycle.
6696
_requestRedraw();
@@ -69,6 +99,11 @@ void RNSkOpenGLCanvasProvider::surfaceDestroyed() {
6999
// destroy the renderer (a unique pointer so the dtor will be called
70100
// immediately.)
71101
_surfaceHolder = nullptr;
102+
if (_jSurfaceTexture) {
103+
JNIEnv *env = facebook::jni::Environment::current();
104+
env->DeleteGlobalRef(_jSurfaceTexture);
105+
_jSurfaceTexture = nullptr;
106+
}
72107
}
73108

74109
void RNSkOpenGLCanvasProvider::surfaceSizeChanged(int width, int height) {

Diff for: ‎packages/skia/android/cpp/rnskia-android/RNSkOpenGLCanvasProvider.h

+5-2
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@
55
#include <memory>
66

77
#include "RNSkView.h"
8-
#include "SkiaOpenGLSurfaceFactory.h"
8+
#include "WindowContext.h"
9+
910
#include <android/native_window.h>
1011

1112
namespace RNSkia {
@@ -33,7 +34,9 @@ class RNSkOpenGLCanvasProvider
3334
void surfaceSizeChanged(int width, int height);
3435

3536
private:
36-
std::unique_ptr<WindowSurfaceHolder> _surfaceHolder = nullptr;
37+
std::unique_ptr<WindowContext> _surfaceHolder = nullptr;
3738
std::shared_ptr<RNSkPlatformContext> _platformContext;
39+
jobject _jSurfaceTexture = nullptr;
40+
jmethodID _updateTexImageMethod = nullptr;
3841
};
3942
} // namespace RNSkia

0 commit comments

Comments
 (0)