Skip to content

Commit 6fbaeab

Browse files
wcandillonwcandillon
authored
fix(🤖): Fix serious Android threading issue (#2749)
Every offscreen surfaces where sharing the OpenGL context even though they may be created from different thread. This can easily cause crashes, especially in viewRef.makeImageSnapshot which is using an offscreen surface. We sanitized the OpenGLContext to not have anything shared across threads.
1 parent 53a8db5 commit 6fbaeab

15 files changed

+571
-621
lines changed

‎packages/skia/android/CMakeLists.txt

+2-1
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ if(SK_GRAPHITE)
5454
else()
5555
add_definitions(-DSK_GL -DSK_GANESH)
5656
set(BACKEND_SOURCES
57-
"${PROJECT_SOURCE_DIR}/cpp/rnskia-android/SkiaOpenGLSurfaceFactory.cpp"
57+
"${PROJECT_SOURCE_DIR}/cpp/rnskia-android/OpenGLWindowContext.cpp"
5858
"${PROJECT_SOURCE_DIR}/cpp/rnskia-android/GrAHardwareBufferUtils.cpp"
5959
)
6060
endif()
@@ -74,6 +74,7 @@ add_library(
7474
"${PROJECT_SOURCE_DIR}/cpp/jni/JniSkiaManager.cpp"
7575

7676
"${PROJECT_SOURCE_DIR}/cpp/jni/JniPlatformContext.cpp"
77+
"${PROJECT_SOURCE_DIR}/cpp/rnskia-android/opengl/Error.cpp"
7778
"${PROJECT_SOURCE_DIR}/cpp/rnskia-android/RNSkOpenGLCanvasProvider.cpp"
7879
"${PROJECT_SOURCE_DIR}/cpp/rnskia-android/AHardwareBufferUtils.cpp"
7980
"${PROJECT_SOURCE_DIR}/cpp/rnskia-android/RNSkAndroidVideo.cpp"

‎packages/skia/android/cpp/rnskia-android/OpenGLContext.h

+122-11
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,18 @@
11
#pragma once
22

3-
#include "SkiaOpenGLSurfaceFactory.h"
4-
#include "WindowContext.h"
3+
#include "GrAHardwareBufferUtils.h"
4+
#include "OpenGLWindowContext.h"
5+
#include "opengl/Display.h"
56

7+
#include "include/core/SkCanvas.h"
8+
#include "include/core/SkColorSpace.h"
69
#include "include/core/SkSurface.h"
10+
#include "include/gpu/ganesh/GrDirectContext.h"
11+
#include "include/gpu/ganesh/SkImageGanesh.h"
12+
#include "include/gpu/ganesh/gl/GrGLBackendSurface.h"
13+
#include "include/gpu/ganesh/gl/GrGLDirectContext.h"
14+
#include "include/gpu/ganesh/gl/GrGLInterface.h"
15+
#include "src/gpu/ganesh/gl/GrGLDefines.h"
716

817
class OpenGLContext {
918
public:
@@ -16,25 +25,127 @@ class OpenGLContext {
1625
}
1726

1827
sk_sp<SkSurface> MakeOffscreen(int width, int height) {
19-
return RNSkia::SkiaOpenGLSurfaceFactory::makeOffscreenSurface(
20-
&_context, width, height);
28+
auto colorType = kRGBA_8888_SkColorType;
29+
30+
SkSurfaceProps props(0, kUnknown_SkPixelGeometry);
31+
32+
auto result = _ctx->makeCurrent(*_surface);
33+
if (!result) {
34+
return nullptr;
35+
}
36+
37+
// Create texture
38+
auto texture = _directContext->createBackendTexture(
39+
width, height, colorType, skgpu::Mipmapped::kNo, GrRenderable::kYes);
40+
41+
if (!texture.isValid()) {
42+
RNSkia::RNSkLogger::logToConsole(
43+
"couldn't create offscreen texture %dx%d", width, height);
44+
}
45+
46+
struct ReleaseContext {
47+
GrDirectContext *directContext;
48+
GrBackendTexture texture;
49+
};
50+
51+
auto releaseCtx = new ReleaseContext{.directContext = _directContext.get(),
52+
.texture = texture};
53+
54+
// Create a SkSurface from the GrBackendTexture
55+
return SkSurfaces::WrapBackendTexture(
56+
_directContext.get(), texture, kTopLeft_GrSurfaceOrigin, 0, colorType,
57+
nullptr, &props,
58+
[](void *addr) {
59+
auto releaseCtx = reinterpret_cast<ReleaseContext *>(addr);
60+
releaseCtx->directContext->deleteBackendTexture(releaseCtx->texture);
61+
delete releaseCtx;
62+
},
63+
releaseCtx);
2164
}
2265

23-
sk_sp<SkImage> MakeImageFromBuffer(void *buffer) {
24-
return RNSkia::SkiaOpenGLSurfaceFactory::makeImageFromHardwareBuffer(
25-
&_context, buffer);
66+
sk_sp<SkImage> MakeImageFromBuffer(void *buffer,
67+
bool requireKnownFormat = false) {
68+
#if __ANDROID_API__ >= 26
69+
const AHardwareBuffer *hardwareBuffer =
70+
static_cast<AHardwareBuffer *>(buffer);
71+
RNSkia::DeleteImageProc deleteImageProc = nullptr;
72+
RNSkia::UpdateImageProc updateImageProc = nullptr;
73+
RNSkia::TexImageCtx deleteImageCtx = nullptr;
74+
75+
AHardwareBuffer_Desc description;
76+
AHardwareBuffer_describe(hardwareBuffer, &description);
77+
GrBackendFormat format;
78+
switch (description.format) {
79+
// TODO: find out if we can detect, which graphic buffers support
80+
// GR_GL_TEXTURE_2D
81+
case AHARDWAREBUFFER_FORMAT_R8G8B8A8_UNORM:
82+
format = GrBackendFormats::MakeGL(GR_GL_RGBA8, GR_GL_TEXTURE_EXTERNAL);
83+
case AHARDWAREBUFFER_FORMAT_R16G16B16A16_FLOAT:
84+
format = GrBackendFormats::MakeGL(GR_GL_RGBA16F, GR_GL_TEXTURE_EXTERNAL);
85+
case AHARDWAREBUFFER_FORMAT_R5G6B5_UNORM:
86+
format = GrBackendFormats::MakeGL(GR_GL_RGB565, GR_GL_TEXTURE_EXTERNAL);
87+
case AHARDWAREBUFFER_FORMAT_R10G10B10A2_UNORM:
88+
format = GrBackendFormats::MakeGL(GR_GL_RGB10_A2, GR_GL_TEXTURE_EXTERNAL);
89+
case AHARDWAREBUFFER_FORMAT_R8G8B8_UNORM:
90+
format = GrBackendFormats::MakeGL(GR_GL_RGB8, GR_GL_TEXTURE_EXTERNAL);
91+
#if __ANDROID_API__ >= 33
92+
case AHARDWAREBUFFER_FORMAT_R8_UNORM:
93+
format = GrBackendFormats::MakeGL(GR_GL_R8, GR_GL_TEXTURE_EXTERNAL);
94+
#endif
95+
default:
96+
if (requireKnownFormat) {
97+
format = GrBackendFormat();
98+
} else {
99+
format = GrBackendFormats::MakeGL(GR_GL_RGBA8, GR_GL_TEXTURE_EXTERNAL);
100+
}
101+
}
102+
103+
auto backendTex = RNSkia::MakeGLBackendTexture(
104+
_directContext.get(), const_cast<AHardwareBuffer *>(hardwareBuffer),
105+
description.width, description.height, &deleteImageProc,
106+
&updateImageProc, &deleteImageCtx, false, format, false);
107+
if (!backendTex.isValid()) {
108+
RNSkia::RNSkLogger::logToConsole(
109+
"Failed to convert HardwareBuffer to OpenGL Texture!");
110+
return nullptr;
111+
}
112+
sk_sp<SkImage> image = SkImages::BorrowTextureFrom(
113+
_directContext.get(), backendTex, kTopLeft_GrSurfaceOrigin,
114+
kRGBA_8888_SkColorType, kOpaque_SkAlphaType, nullptr, deleteImageProc,
115+
deleteImageCtx);
116+
return image;
117+
#else
118+
throw std::runtime_error(
119+
"HardwareBuffers are only supported on Android API 26 or higher! Set "
120+
"your minSdk to 26 (or higher) and try again.");
121+
#endif
26122
}
27123

28124
std::unique_ptr<RNSkia::WindowContext> MakeWindow(ANativeWindow *window,
29125
int width, int height) {
30-
return RNSkia::SkiaOpenGLSurfaceFactory::makeContext(&_context, window,
31-
width, height);
126+
return std::make_unique<RNSkia::OpenGLWindowContext>(
127+
_config, _display.get(), _ctx.get(), _directContext.get(), window,
128+
width, height);
32129
}
33130

34131
private:
35-
RNSkia::SkiaOpenGLContext _context;
132+
EGLConfig _config;
133+
std::unique_ptr<RNSkia::Display> _display;
134+
std::unique_ptr<RNSkia::Context> _ctx;
135+
std::unique_ptr<RNSkia::Surface> _surface;
136+
sk_sp<GrDirectContext> _directContext;
36137

37138
OpenGLContext() {
38-
RNSkia::SkiaOpenGLHelper::createSkiaDirectContextIfNecessary(&_context);
139+
_display = std::make_unique<RNSkia::Display>();
140+
_config = _display->chooseConfig();
141+
_ctx = _display->makeContext(_config, nullptr);
142+
_surface = _display->makePixelBufferSurface(_config, 1, 1);
143+
_ctx->makeCurrent(*_surface);
144+
auto backendInterface = GrGLMakeNativeInterface();
145+
_directContext = GrDirectContexts::MakeGL(backendInterface);
146+
147+
if (_directContext == nullptr) {
148+
throw std::runtime_error("GrDirectContexts::MakeGL failed");
149+
}
39150
}
40151
};

‎packages/skia/android/cpp/rnskia-android/OpenGLWindowContext.cpp

+73
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
#include "OpenGLWindowContext.h"
2+
#include "GrAHardwareBufferUtils.h"
3+
4+
#pragma clang diagnostic push
5+
#pragma clang diagnostic ignored "-Wdocumentation"
6+
7+
#include "include/gpu/ganesh/SkImageGanesh.h"
8+
#include "include/gpu/ganesh/gl/GrGLBackendSurface.h"
9+
#include "src/gpu/ganesh/gl/GrGLDefines.h"
10+
11+
#pragma clang diagnostic pop
12+
13+
namespace RNSkia {
14+
15+
sk_sp<SkSurface> OpenGLWindowContext::getSurface() {
16+
if (_skSurface == nullptr) {
17+
18+
struct ReleaseContext {
19+
std::unique_ptr<Surface> surface = nullptr;
20+
};
21+
22+
auto releaseCtx = new ReleaseContext();
23+
releaseCtx->surface = _display->makeWindowSurface(_config, _window);
24+
_surface = releaseCtx->surface.get();
25+
26+
// Now make this one current
27+
_context->makeCurrent(*releaseCtx->surface);
28+
29+
// Set up parameters for the render target so that it
30+
// matches the underlying OpenGL context.
31+
GrGLFramebufferInfo fboInfo;
32+
33+
// We pass 0 as the framebuffer id, since the
34+
// underlying Skia GrGlGpu will read this when wrapping the context in the
35+
// render target and the GrGlGpu object.
36+
fboInfo.fFBOID = 0;
37+
fboInfo.fFormat = 0x8058; // GL_RGBA8
38+
39+
GLint stencil;
40+
glGetIntegerv(GL_STENCIL_BITS, &stencil);
41+
42+
GLint samples;
43+
glGetIntegerv(GL_SAMPLES, &samples);
44+
45+
auto colorType = kN32_SkColorType;
46+
47+
auto maxSamples =
48+
_directContext->maxSurfaceSampleCountForColorType(colorType);
49+
50+
if (samples > maxSamples) {
51+
samples = maxSamples;
52+
}
53+
54+
auto renderTarget = GrBackendRenderTargets::MakeGL(_width, _height, samples,
55+
stencil, fboInfo);
56+
57+
SkSurfaceProps props(0, kUnknown_SkPixelGeometry);
58+
59+
60+
// Create surface object
61+
_skSurface = SkSurfaces::WrapBackendRenderTarget(
62+
_directContext, renderTarget, kBottomLeft_GrSurfaceOrigin, colorType,
63+
nullptr, &props,
64+
[](void *addr) {
65+
auto releaseCtx = reinterpret_cast<ReleaseContext *>(addr);
66+
delete releaseCtx;
67+
},
68+
reinterpret_cast<void *>(releaseCtx));
69+
}
70+
return _skSurface;
71+
}
72+
73+
} // namespace RNSkia

‎packages/skia/android/cpp/rnskia-android/OpenGLWindowContext.h

+74
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
#pragma once
2+
3+
#include "RNSkLog.h"
4+
5+
#include <fbjni/fbjni.h>
6+
#include <jni.h>
7+
8+
#include <android/native_window_jni.h>
9+
#include <android/surface_texture.h>
10+
#include <android/surface_texture_jni.h>
11+
#include <condition_variable>
12+
#include <memory>
13+
#include <thread>
14+
#include <unordered_map>
15+
16+
#include "WindowContext.h"
17+
#include "opengl/Display.h"
18+
19+
#pragma clang diagnostic push
20+
#pragma clang diagnostic ignored "-Wdocumentation"
21+
22+
#include "include/core/SkCanvas.h"
23+
#include "include/core/SkColorSpace.h"
24+
#include "include/core/SkSurface.h"
25+
#include "include/gpu/ganesh/GrBackendSurface.h"
26+
#include "include/gpu/ganesh/GrDirectContext.h"
27+
#include "include/gpu/ganesh/SkSurfaceGanesh.h"
28+
#include "include/gpu/ganesh/gl/GrGLInterface.h"
29+
30+
#pragma clang diagnostic pop
31+
32+
namespace RNSkia {
33+
34+
class OpenGLWindowContext : public WindowContext {
35+
public:
36+
OpenGLWindowContext(EGLConfig &config, Display *display, Context *context,
37+
GrDirectContext *directContext, ANativeWindow *window,
38+
int width, int height)
39+
: _config(config), _display(display), _directContext(directContext),
40+
_context(context), _window(window), _width(width), _height(height) {}
41+
42+
~OpenGLWindowContext() { ANativeWindow_release(_window); }
43+
44+
sk_sp<SkSurface> getSurface() override;
45+
46+
void present() override {
47+
_context->makeCurrent(*_surface);
48+
_directContext->flushAndSubmit();
49+
_surface->Present();
50+
}
51+
52+
void resize(int width, int height) override {
53+
_skSurface = nullptr;
54+
_width = width;
55+
_height = height;
56+
}
57+
58+
int getWidth() override { return _width; };
59+
60+
int getHeight() override { return _height; };
61+
62+
private:
63+
ANativeWindow *_window;
64+
sk_sp<SkSurface> _skSurface = nullptr;
65+
EGLConfig _config;
66+
Context *_context;
67+
Surface* _surface;
68+
Display *_display;
69+
GrDirectContext *_directContext;
70+
int _width = 0;
71+
int _height = 0;
72+
};
73+
74+
} // namespace RNSkia

‎packages/skia/android/cpp/rnskia-android/RNSkOpenGLCanvasProvider.cpp

-1
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,6 @@ bool RNSkOpenGLCanvasProvider::renderToCanvas(
7171
return false;
7272
}
7373
}
74-
7574
return false;
7675
}
7776

0 commit comments

Comments
 (0)